Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup geoipRequired and uaRequired #31173

Merged
merged 2 commits into from
Feb 18, 2019

Conversation

kaiyan-sheng
Copy link
Contributor

In Elasticsearch 7.0 the geoip and the user_agent processor as shipped by default. The step to install geoip and the user_agent plugins is removed in #30866. This PR is to remove code that uses geoipRequired and uaRequired from Kibana.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@kaiyan-sheng
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kaiyan-sheng kaiyan-sheng added the Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services label Feb 15, 2019
@kaiyan-sheng kaiyan-sheng self-assigned this Feb 15, 2019
@kaiyan-sheng kaiyan-sheng requested a review from ruflin February 15, 2019 12:56
@ruflin ruflin requested a review from ycombinator February 15, 2019 21:00
@ruflin
Copy link
Contributor

ruflin commented Feb 15, 2019

@ycombinator Could you review this one, I remember you created / touched this code? :-)

Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes LGTM.

Just wanted to note that I tried out master before I tried this PR and I couldn't find any tutorial that had these GeoIP or UA plugin instructions in it. I believe that's because all tutorials using Filebeat instructions were setting geoipRequired and uaRequired to false before passing them to the instruction generation functions.

So, in effect, the tutorials looked the same to me before and after this PR. Hopefully this is expected?

@kaiyan-sheng
Copy link
Contributor Author

@ycombinator Yes thank you for the review! #30866 changed all geoipRequired and uaRequired to False and this PR is just a cleanup.

@kaiyan-sheng kaiyan-sheng merged commit 5f196a9 into elastic:master Feb 18, 2019
@kaiyan-sheng kaiyan-sheng deleted the remove_geoip_useragent branch February 18, 2019 00:10
ycombinator pushed a commit to ycombinator/kibana that referenced this pull request Apr 5, 2019
* Cleanup geoipRequired and uaRequired

* Remove geoip and user-agent plugins in chinese translations
ycombinator pushed a commit to ycombinator/kibana that referenced this pull request Apr 5, 2019
* Cleanup geoipRequired and uaRequired

* Remove geoip and user-agent plugins in chinese translations
ycombinator pushed a commit to ycombinator/kibana that referenced this pull request Apr 5, 2019
* Cleanup geoipRequired and uaRequired

* Remove geoip and user-agent plugins in chinese translations
ycombinator added a commit that referenced this pull request Apr 5, 2019
Backports the following commits to 7.0:
 - Cleanup geoipRequired and uaRequired  (#31173)
ycombinator added a commit that referenced this pull request Apr 5, 2019
Backports the following commits to 7.x:
 - Cleanup geoipRequired and uaRequired  (#31173)
ycombinator added a commit that referenced this pull request Apr 5, 2019
Backports the following commits to 6.7:
 - Cleanup geoipRequired and uaRequired  (#31173)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants